Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protorev: Use hooks instead of message parsing to trigger backruns #5045

Merged
merged 76 commits into from
Jun 6, 2023

Conversation

NotJeremyLiu
Copy link
Contributor

@NotJeremyLiu NotJeremyLiu commented May 2, 2023

What is the purpose of the change

  • We've concluded a hook-based approach to protorev will be the best approach to keep protorev up to date given several new message types being introduced that allow for swaps.
  • Previously, protorev would case on specific message types for a transaction, and extract the pools that were swapped against to check for backrun potential.
  • This PR changes this approach to now plug protorev into the GAMM and Concentrated liquidity hooks, store the necessary swap information in the hook, and then read from that store in the protorev posthandler.
  • We also add logic to support join/exit pool hooks for the GAMM module (treating single sided joins/exits as swaps), and pool creation (updating highest liquidity pools at creation of a new pool)

Brief Changelog

  • Add new kvstore for SwapsToBackrun and add setters and getters
  • Add hooks.go file that plugs into GAMM and ConcratedLiquidity pool hooks and stores/updates necessary protorev kvstores as appropriate based on the hook
    • Add SwapsToBackrun when triggered by a swap hook or join/exit pool hook that also swaps
    • Update highest liquidity pools at pool creation for GAMM and at initial liquidity position for CL (triggered by hook)
  • Add SwapsToBackrun delete logic in postahandler without using gas

Testing and Verifying

  • New tests added for all protorev hook logic in hooks_test.go
  • Updated previous tests to use updated methods and no longer rely on messages as input
  • Updated epoch_hook_test.go to account for new concentrated liquidity pool testing changes that sets default coins when creating a pool with a position range

Documentation and Release Note

  • Added changelog entry

@github-actions github-actions bot added C:app-wiring Changes to the app folder C:x/concentrated-liquidity C:x/pool-incentives C:x/twap Changes to the twap module labels May 2, 2023
@github-actions
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you!

@github-actions github-actions bot added the Stale label May 17, 2023
@github-actions github-actions bot removed the Stale label May 19, 2023
- Limit len(coins) = 1 due to an exit pool message giving all denoms as exitdenoms (and potentially future behavior of other modules)
@NotJeremyLiu NotJeremyLiu requested a review from p0mvn June 5, 2023 19:37
x/protorev/keeper/hooks.go Outdated Show resolved Hide resolved
Copy link
Member

@p0mvn p0mvn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work on expanding the tests! Looks great. I will do a short final pass once all comments are addressed prior to merging

Copy link
Member

@p0mvn p0mvn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent work @NotJeremyLiu

@p0mvn p0mvn merged commit c6697a5 into main Jun 6, 2023
@p0mvn p0mvn deleted the jl/protorev-hook-based-logic branch June 6, 2023 14:35
@github-actions github-actions bot mentioned this pull request Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:app-wiring Changes to the app folder protorev All things related to x/protorev V:state/breaking State machine breaking PR
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants